Harden launcher preflight and port handling#357
Conversation
|
Retargeted this PR to |
| "#!/bin/sh\n" | ||
| f"cd {shlex.quote(BASE_DIR)} || exit 1\n" | ||
| f"if curl -fsS {shlex.quote(browser_url)} >/dev/null 2>&1 " | ||
| f"&& curl -fsS {shlex.quote(backend_url)} >/dev/null 2>&1; then\n" |
There was a problem hiding this comment.
Problem: The -f flag makes curl "fail" (exit non-zero) whenever the server replies with a 4xx status. But CraftBot's backend normally returns a 4xx on a plain GET /, even when it's perfectly healthy, run.py itself treats anything under 500 as "backend is up".
Result: The health check thinks the bot is down when it's actually running. So double-clicking the desktop shortcut restarts the whole service instead of just opening the browser.
Fix: Drop -f so any HTTP response counts as "up" (matching run.py), or point the check at a route that actually returns 200.
| content = f"#!/bin/sh\nopen '{BROWSER_URL}'\n" | ||
| backend_url = _backend_url(extra_args) | ||
| restart_args = _port_args(extra_args) | ||
| start_cmd = shlex.join([_python_exe(), "craftbot.py", "start"] + restart_args) |
There was a problem hiding this comment.
Problem: This assumes craftbot.py sits next to Python. In a frozen/installed macOS build, there's no craftbot.py file it's baked into the binary so the restart command has nothing to run.
Why it matters: Because of Comment on Line 878, this broken restart branch is exactly the one that fires when the bot is already running. So the two bugs stack: wrong branch chosen, and that branch fails.
Fix: Only use craftbot.py start when running from source (not IS_FROZEN); in a frozen build, call the installed executable instead.
|
Could you also point the PR to V1.4.1? Thanks |
2 participants
Summary
--frontend-port/--backend-portinstead of hardcoding the backend port.craftbot.py,run.py, andinstaller/api.pyshare the same value.Response to review feedback
@korivi-CraftOS this addresses the two follow-up items from #347:
runtime_preflight.py: probe infrastructure failures now warn and continue.TimeoutExpired, subprocess exceptions, nonzero probe exits, missing sentinel output, malformed JSON, and non-list JSON are all treated as inconclusive. Startup only exits when the probe succeeds and returns a concrete list of missing imports.craftbot.py: frontend/backend URLs now derive from the passed port flags, including both--backend-port 8123and--backend-port=8123forms. The macOS shortcut uses the derived backend URL for its health check.CRAFTBOT IS READYnow lives in top-levelstartup_constants.py, which both the installer and agent-side code can import.Validation
/usr/local/bin/python3.10 -m pytest tests/test_run_dependency_check.py tests/test_craftbot_service.py tests/test_startup_constants_usage.py tests/test_model_factory.py -q->15 passed10/10successfully before opening this PR./usr/local/bin/python3.10 -m py_compile craftbot.py run.py app/main.py app/runtime_preflight.py installer/api.py startup_constants.py tests/test_craftbot_service.py tests/test_run_dependency_check.py tests/test_startup_constants_usage.py-> passedgit diff --check origin/main...HEAD-> passedrun.py,python -m app.main, andcraftbot.py start --cliall exited nonzero with clear missing-dependency messaging; no traceback, falseCRAFTBOT STARTED, or stale PID./usr/local/bin/python3.10:10/10successful./usr/local/bin/python3.10:3/3start/status/stop cycles successful.Repo-wide checks still have unrelated existing failures outside this PR's changed files:
/usr/local/bin/python3.10 -m pytest tests -q->101 passed, 4 failed, 16 deselected; the failures are intests/test_trigger_router_and_parking.pybecauseFakeLLM.generate_response_async()does not acceptprompt_name./usr/local/bin/python3.10 -m pytest -q-> collection errors in generated/template Living UI test directories (modelsimport andtests.conftestimport-path mismatch).